<!DOCTYPE HTML PUBLIC "-//SoftQuad//DTD HTML 3.2 + extensions for HoTMetaL PRO 3.0(U) 19961211//EN" "hmpro3.dtd">
<html>
<head>
  <meta http-equiv="content-type" content="text/html; charset=iso-8859-1">
  <title></title>
</head>

<body>
<h1>C++ Pitfalls </h1>

<h2><b>Cay S. Horstmann</b></h2>

<h3><i>Department of Computer Science</i></h3>

<p>San Jose State University </p>

<p>San Jose, CA 95192-0249 </p>

<h4><tt>cay\x40horstmann.com </tt> </h4>

<p>Copyright (C) Cay S. Horstmann 1997</p>
<hr>

<h3><b>What is a pitfall? </b></h3>

<p>C++ code that </p>
<ul>
  <li>compiles </li>
  <li>links </li>
  <li>runs </li>
  <li>does something different than you expect </li>
</ul>

<p><i>Example: </i> </p>
<pre>             if (-0.5 &lt;= x &lt;= 0.5) return 0;<br> </pre>

<p><i>Pitfall: </i></p>
<pre>              if (-0.5 &lt;= x &lt;= 0.5) return 0;<br></pre>

<p>This expression does<i>not</i> test the mathematical condition </p>
<pre>              -1.5 &lt;= x &lt;= 1.5<br></pre>

<p>Instead, it first computes <tt>-0.5 &lt;= x</tt>, which is 0 or 1, and then
compares the result with 0.5. </p>

<p><i>Moral:</i> Even though C++ now has a <tt>bool</tt> type, Booleans are
still freely convertible to <tt>int</tt>.</p>

<p>Since <tt>bool-&gt;int</tt> is allowed as a conversion, the compiler cannot
check the validity of expressions. In contrast, the Java compiler would flag
this statement as an error. </p>
<hr>

<h3><b>Constructor pitfalls </b></h3>

<p><i>Example: </i></p>
<pre>              int main()<br>              {  string a("Hello");<br>                 string b();<br>                 string c = string("World");<br>                 // ...<br>                 return 0;<br>              }  <br> </pre>

<p><i>Pitfall: </i></p>
<pre>              string b();<br></pre>

<p>This expression does not construct an object <tt>b</tt> of type
<tt>string</tt>. Instead, it is the prototype for a function <tt>b</tt> with no
arguments and return type <tt>string</tt>. </p>

<p><i>Moral:</i> Remember to omit the <tt>( )</tt> when invoking the default
constructor. </p>

<p>The C feature of declaring a function in a local scope is worthless since it
lies about the true scope. Most programmers place all prototypes in header
files. But even a worthless feature that you never use can haunt you. </p>
<hr>

<p><i>Example: </i></p>
<pre>              template&lt;typename T&gt;<br>              class Array<br>              {<br>              public:<br>                 Array(int size);<br>                 T&amp; operator[](int);<br>                 Array&lt;T&gt;&amp; operator=(const Array&lt;T&gt;&amp;);<br>                 // ...<br>              }; <br><br>              int main()<br>              {  Array&lt;double&gt; a(10);<br>                 a[0] = 0; a[1] = 1; a[2] = 4; <br>                 a[3] = 9; a[4] = 16;<br>                 a[5] = 25; a = 36; a[7] = 49; <br>                 a[8] = 64; a[9] = 81;<br>                 // ...<br>                 return 0;<br>              }  <br></pre>

<p><i>Pitfall: </i></p>
<pre>              a = 36;<br></pre>

<p>Surprisingly, it compiles: </p>
<pre>              a = Array&lt;double&gt;(36);<br></pre>

<p><tt>a</tt> is replaced with a new array of 36 numbers. </p>

<p><i>Moral:</i> Constructors with one argument serve double duty as type
conversions. </p>

<p>Avoid constructors with a single integer argument! Use the <tt>explicit</tt>
keyword if you can't avoid them.</p>
<hr>

<p><i>Example: </i></p>
<pre>              template&lt;typename T&gt;<br>              class Array<br>              {<br>              public:<br>                 explicit Array(int size);<br>                 // ...<br>              private:<br>                 T* _data;<br>                 int _size;<br>              }; <br><br>              template&lt;typename T&gt;<br>              Array&lt;T&gt;::Array(int size)<br>              :  _size(size), <br>                 _data(new T(size))<br>              {}<br><br>              int main()<br>              {  Array&lt;double&gt; a(10);<br>                 a[1] = 64; // program crashes<br>                 // ...<br>              }  <br></pre>

<p><i>Pitfall: </i></p>
<pre>              template&lt;typename T&gt;<br>              Array&lt;T&gt;::Array(int size)<br>              :  _size(size), <br>                 _data(new T<b>(</b>size<b>)</b>) // should have been <b>new T[size]</b>
              {}</pre>

<p>Why did it compile? </p>
<pre>              new T(size)<br></pre>

<p>returns a <tt>T*</tt> pointer to a single element of type <tt>T</tt>,
constructed from the integer <tt>size</tt>. </p>
<pre>              new T[size]<br></pre>

<p>returns a <tt>T*</tt> pointer to an array of <tt>size</tt> objects of type
<tt>T</tt>, constructed with the default constructor.</p>

<p><i>Moral:</i> Array/pointer duality is dumb, but unfortunately pervasive in
C and C++. </p>

<p>The Java compiler would catch this--Java, like most programming languages,
supports genuine array types.</p>
<hr>

<p><i>Example: </i></p>
<pre>              template&lt;typename T&gt;<br>              class Array<br>              {<br>              public:<br>                 explicit Array(int size);<br>                 // ...<br>              private:<br>                 T* _data;<br>                 int _capacity;<br>                 int _size;<br>              }; <br><br>              template&lt;typename T&gt;<br>              Array&lt;T&gt;::Array(int size)<br>              :  _size(size), <br>                 _capacity(_size + 10), <br>                 _data(new T[_capacity])<br>              {}<br><br><br>             int main() <br>             {  Array&lt;int&gt; a(100);<br>                . . .<br>                // program starts acting flaky<br>             } <br></pre>

<p><i>Pitfall: </i></p>
<pre>              Array&lt;T&gt;::Array(int size)<br>              :  _size(size), <br>                 _capacity(size + 10), <br><b>                 _data(new T[_capacity])<br></b>              {} <br></pre>

<p>Initialization follows the member declaration order, not the initializer
order! </p>
<pre>          Array&lt;T&gt;::Array(int size)<br>              :  _data(new T[_capacity])<br>                 _capacity(_size + 10), <br>                 _size(size), <br> </pre>

<p><i>Tip:</i> Do not use data members in initializer expressions. </p>
<pre>          Array&lt;T&gt;::Array(int size)<br>              :  _data(new T[size + 10])<br>                 _capacity(size + 10), <br>                 _size(size), <br> </pre>
<hr>

<p><i>Example: </i></p>
<pre>              class Point<br>              {  <br>              public:<br>                 Point(double x = 0, double y = 0);<br>                 // ...<br>              private:<br>                 double _x, _y;<br>              }; <br>              int main()<br>              {  double a, r, x, y;<br>                 // ...<br>                 Point p = (x + r * cos(a), y + r * sin(a));<br>                 // ...<br>                 return 0;<br>              }  <br></pre>

<p><i>Pitfall: </i></p>
<pre>              Point p = (x + r * cos(a), y + r * sin(a));<br></pre>

<p>This should be either <tt></tt></p>
<pre>              Point p(x + r * cos(a), y + r * sin(a));<br></pre>

<p>or </p>
<pre>              Point p = Point(x + r * cos(a), y + r * sin(a));<br></pre>

<p>The expression </p>
<pre>              (x + r * cos(a), y + r * sin(a))<br></pre>

<p>has a legal meaning. The comma operator discards <tt>x + r * cos(a)</tt> and
evaluates<tt>y + r * sin(a)</tt>. The </p>
<pre>              Point(double x = 0, double y = 0)<br></pre>

<p>constructor makes a <tt>Point(y + r * sin(a), 0).</tt></p>

<p><i>Moral:</i> Default arguments can lead to unintended calls. In our case,
the construction<tt>Point(double)</tt> is not reasonable, but the
construction<tt>Point()</tt> is. Only use defaults if all resulting call
patterns are meaningful. </p>
<hr>

<p><i>Example: </i></p>
<pre>              class Shape<br>              {  <br>              public:<br>                 Shape();<br>              private:<br>                 virtual void reset();<br>                 Color _color;<br>              }; <br><br>              class Point : public Shape<br>              {  <br>              public:<br>                 // ...<br>              private:<br>                 double _x, _y;<br>              }; <br><br>              void Shape::reset() { _color = BLACK; }<br><br>              void Point::reset() <br>              {  Shape::reset();<br>                 _x = 0; _y = 0;<br>              }  <br><br>              Shape::Shape() { reset(); }<br></pre>

<p>There is no<tt>Point</tt> constructor--we use the virtual function in the
Shape constructor. </p>

<p><i>Pitfall: </i></p>
<pre>              Shape::Shape() { reset(); }<br>              Point p;<br></pre>

<p>When constructing<tt>Point</tt>, the <tt>Shape::reset()</tt>, not the
<tt>Point::reset()</tt> virtual function is called. Why? </p>

<p><i>Explanation:</i> Virtual functions do not work in constructors. </p>

<p>The<tt>Shape</tt> subobject is constructed before the<tt>Point</tt> object.
Inside the <tt>Shape</tt> constructor, the partially constructed object is
still a <tt>Shape</tt>. </p>
<hr>

<p><i>Example: </i></p>
<pre>              class Shape // an abstract class<br>              {  <br>              public:<br>                 Shape();<br>              private:<br>                 void init();<br>                 virtual void reset() = 0;<br>                 Color _color;<br>              }; <br><br>              Shape::Shape() { init(); }<br>              void Shape::init() { reset(); } <br><br>              class Point : public Shape // a concrete derived class<br>              {  <br>              public:<br>                 virtual void reset();<br>                 // ...<br>              private:<br>                 double _x, _y;<br>              };<br><br>              void Point::reset() { _x = _y = 0; }<br><br></pre>

<p><i>Pitfall: </i></p>
<pre>              int main()<br>              {  Point p; // program crashes<br>                 return 0;<br>              } <br></pre>

<p><i>Explanation:</i> You cannot create an instance of an abstract class (a
class with a pure, <tt>= 0</tt>, virtual function). </p>
<pre>         Shape s; // compile-time error; Shape is abstract</pre>

<p>That's a good thing: if you could, what would happen if you called</p>
<pre>         s.reset(); // reset not defined for shapes</pre>

<p>But...I lied. You <i>can</i> create instances of abstract classes.</p>

<p>When constructing a concrete derived, for a fleeting moment, the base class
exists. If you invoke a pure virtual function before the derived class
constructor has executed, the program terminates.</p>
<hr>

<h3><b>Destructor pitfalls </b></h3>

<p><i>Example:</i> </p>
<pre>              class Employee<br>              {<br>              public:<br>                 Employee(string name);<br>                 virtual void print() const;<br>              private:<br>                 string _name;<br>              };<br><br>              class Manager : public Employee<br>              {<br>              public:<br>                 Manager(string name, string dept);<br>                 virtual void print() const;<br>              private:<br>                 string _dept;<br>              };<br><br>              int main()<br>              {  Employee* staff[10];<br>                 staff[0] = new Employee("Harry Hacker");<br>                 staff[1] = new Manager("Joe Smith", "Sales");<br>                 // ...<br>                 for (int i = 0; i &lt; 10; i++) <br>                    staff[i]-&gt;print();<br>                 for (int i = 0; i &lt; 10; i++) <br>                    delete staff[i];<br>                 return 0;<br>              }<br></pre>

<p>Where is the memory leak? </p>

<p><i>Pitfall: </i></p>
<pre>              delete staff[i];<br></pre>

<p>destroys all objects with<tt>~Employee()</tt>. The<tt>_dept</tt> strings of
the <tt>Manager</tt> objects are never destroyed. </p>

<p><i>Moral:</i> A class from which you derive must have a virtual destructor.
</p>
<hr>

<p><i>Example:</i> <tt></tt></p>
<pre>              class Employee<br>              {<br>              public:<br>                 Employee(string name);<br>                 virtual void print() const;<br>                 virtual ~Employee(); // &lt;-----<br>              private:<br>                 string _name;<br>              };<br><br>              class Employee<br>              {<br>              public:<br>                 Employee(string name);<br>              private:<br>                 string _name;<br>              };<br><br>              class Manager<br>              {<br>              public:<br>                 Manager(string name, string sname);<br>                 ~Manager();<br>              private:<br>                 Employee* _secretary;<br>              }<br><br>              Manager::Manager(string name, string sname)<br>              :  Employee(name),<br>                 _secretary(new Employee(sname))<br>              {} <br><br>              Manager::~Manager() { delete _secretary; }<br></pre>

<p>What is wrong with the <tt>Manager</tt> class? </p>

<p><i>Pitfall: </i></p>
<pre>              int main()<br>              {  Manager m1 = Manager("Sally Smith", <br>                    "Joe Barnes");<br>                 Manager m2 = m1;<br>                 // ...<br>              }<br></pre>

<p>The destructors of both <tt>m1</tt> and <tt>m2</tt> will delete the same
<tt>Employee</tt> object. </p>

<p><i>Moral:</i> A class with a destructor needs a copy constructor
<tt></tt></p>
<pre>              Manager::Manager(const Manager&amp;) <br></pre>

<p>and an assignment operator </p>
<pre>              Manager&amp; Manager::operator=(const Manager&amp;). <br></pre>

<p><i>The Big 3: It's not just a good idea--it's the law</i> (Marshall
Cline)</p>
<hr>

<h3><b>Inheritance pitfalls </b></h3>

<p><i>Example: </i></p>
<pre>              class Employee<br>              {<br>              public:<br>                 Employee(string name, string dept);<br>                 virtual void print() const;<br>                 string dept() const;<br>              private:<br>                 string _name;<br>                 string _dept;<br>              };<br><br>              class Manager : public Employee<br>              {<br>              public:<br>                 Manager(string name, string dept);<br>                 virtual void print() const;<br>              private:<br>                 // ...<br>              };<br><br>              void Employee::print() const<br>              {  cout &lt;&lt; _name &lt;&lt; endl;<br>              }  <br><br>              void Manager::print() const<br>              {  print(); // print base class<br>                 cout &lt;&lt; dept() &lt;&lt; endl;<br>              }  <br></pre>

<p><i>Pitfall: </i></p>
<pre>              void Manager::print() const<br>              {  print(); // print base class<br>                 cout &lt;&lt; dept() &lt;&lt; endl;<br>              }  <br></pre>

<p>Despite what the comment says,<tt>print()</tt> selects the print operation
of the<tt>Manager</tt> class. In contrast, <tt>dept()</tt> selects the
operation of the<tt>Employee</tt> class since<tt>Manager</tt> does not redefine
it. </p>

<p><i>Moral:</i> When calling a base class operation in a derived class
operation of the same name, use scope resolution: </p>
<pre>              void Manager::print() const<br>              {  <b>Employee::</b>print(); // print base class<br>                 cout &lt;&lt; dept() &lt;&lt; endl;<br>              }  <br></pre>
<hr>

<p><i>Example: </i></p>
<pre>              void Manager::print() const<br>              {  Employee:print(); // print base class<br>                 cout &lt;&lt; dept() &lt;&lt; endl;<br>              }  <br></pre>

<p><i>Pitfall: </i></p>
<pre>              Employee<b>:</b>print(); <br></pre>

<p>It should be </p>
<pre>              Employee::print(); <br></pre>

<p>But why does it compile?<tt>Employee:</tt> is a goto label! </p>

<p><i>Moral:</i>Even language features that you never use can bite you! </p>
<hr>

<p><i>Example: </i></p>
<pre>              class Employee<br>              {<br>              public:<br>                 void raise_salary(double by_percent);<br>                 // ...<br>              };<br><br>              class Manager : public Employee<br>              {<br>              public:<br>                 // ...<br>              };<br><br>              void make_them_happy(Employee* e, int ne)<br>              {  for (int i = 0; i &lt; ne; i++)<br>                    e[i].raise_salary(0.10);<br>              }     <br><br>              int main()<br>              {  Employee e[20];<br>                 Manager m[5];<br>                 m[0] = Manager("Joe Bush", "Sales");<br>                 // ...<br>                 make_them_happy(e, 20);<br>                 make_them_happy(m + 1, 4); // let's skip Joe<br>                 return 0;<br>              }<br></pre>

<p><i>Pitfall: </i></p>
<pre>              void make_them_happy(Employee* e, int ne);<br>              Manager m[5];<br>              make_them_happy(m + 1, 4);<br></pre>

<p>Why does it compile? </p>

<p>The type of <tt>m + 1</tt> is<tt>Manager*.</tt> Because of inheritance, a
<tt>Manager*</tt> is convertible to an <tt>Employee*</tt> base class pointer.
<tt>make_them_happy </tt>receives an<tt>Employee*</tt>. Everyone is happy. </p>

<p>What is the problem? </p>

<p>The array computation <tt>e[i]</tt> computes an offset of
<tt>i*sizeof(Employee)</tt>. </p>

<p><i>Moral:</i> Pointers are overused in C++. Here we see two interpretations
of an<tt>Employee* e.</tt> </p>
<ol>
  <li><tt>e</tt> points to either an<tt>Employee</tt> or a derived class
    object, such as a <tt>Manager.</tt></li>
  <li><tt>e</tt> points to either an<tt>Employee</tt> or a bunch
    of<tt>Employee</tt> objects, stacked up in an array.</li>
</ol>

<p>These two interpretations are <i>incompatible.</i> Mixing them leads to
runtime errors. However, the <i>intention</i> of the programmer is hidden to
the compiler since both ideas are expressed by the same construct--a pointer.
</p>
<hr>

<h3></h3>

<p><i>Example: </i></p>
<pre>              class Employee<br>              {<br>              public:<br>                 Employee(char name[]);<br>                 Employee(const Employee&amp; b);<br>                 ~Employee();<br>                 Employee&amp; operator=(const Employee&amp; b);<br>                 . . .<br>              private:<br>                 char* _name;<br>              };<br><br>              class Manager : public Employee<br>              {<br>              public:<br>                 Manager(char name[], char dept[]);<br>                 Manager(const Manager&amp; b);<br>                 ~Manager();<br>                 Manager&amp; operator=(const Manager&amp; b);<br>                 . . .<br>              private:<br>                 char* _dept;<br>              };<br><br>              Manager::Manager(const Manager&amp; b)<br>              : _dept(new char[strlen(b._dept) + 1])<br>              {  strcpy(b._dept, _dept);<br>              }<br><br>              Manager::~Manager()<br>              {  delete[] _dept;<br>              }<br><br>              Manager&amp; Manager::operator=(const Manager&amp; b)<br>              {  if (this == &amp;b) return *this;<br>                 delete[] _dept;<br>                 _dept = new char[strlen(b._dept) + 1];<br>                 strcpy(b._dept, _dept);<br>                 return *this;<br>              }<br></pre>

<p><i>Pitfall: </i></p>
<pre>              Manager&amp; Manager::operator=(const Manager&amp; b)<br>              {  if (this == &amp;b) return *this;<br>                 delete[] _dept;<br>                 _dept = new char[strlen(b._dept) + 1];<br>                 strcpy(b._dept, _dept);<br>                 return *this;<br>              }<br>  <br></pre>

<p>Constructors and destructors automatically call the base constructors and
destructors. But <tt>operator=</tt> does <i>not</i> automatically invoke the
<tt>operator=</tt> of the base class. </p>

<p><i>Moral:</i> When redefining <tt>operator=</tt> in a derived class,
explicitly call <tt>operator=</tt> of the base class: </p>
<pre>              Manager&amp; Manager::operator=(const Manager&amp; b)<br>              {  if (this == &amp;b) return *this;<br>                 <b>Employee::operator=(b);</b>
                 delete[] _dept;
                 _dept = new char[strlen(b._dept) + 1];
                 strcpy(b._dept, _dept);
                 return *this;
              }

</pre>

<p>Thanks to Johann Deneux for pointing out another pitfall: The copy
constructor for <code>Manager</code> is wrong. It wants to call the default
constructor for <code>Employee</code>, but there isn't one. And, of course, it
wouldn't be appropriate to call it if there was one. A corrected version is</p>
<pre><br>              Manager::Manager(const Manager&amp; b)<br>              : <strong>Employee(*this)</strong>, _dept(new char[strlen(b._dept) + 1])<br>              {  strcpy(b._dept, _dept);<br>              }<br><br></pre>
<hr>

<h3><b>Stream pitfalls </b></h3>

<p><i>Example:</i> </p>
<pre>              list&lt;int&gt; a;<br>              while (!cin.eof())<br>              {  int x;<br>                 cin &gt;&gt; x;<br>                 if (!cin.eof()) a.push_back(x);<br>              }<br></pre>

<p><i>Pitfall: </i></p>

<p><tt></tt></p>
<pre>              while (!cin.eof())<br>              {  // ...<br>              }<br></pre>

<p>This may be an infinite loop. If the stream state turns to <i>fail</i>, the
end of file will never be reached. </p>

<p>The stream state will be set to <i>fail</i> if a non-digit is encountered
when trying to read an integer. </p>

<p><i>Moral:</i> <tt>eof()</tt> is <i>only useful in combination with</i>
<tt>fail()</tt>, to find out whether EOF was the cause for failure</p>
<hr>

<p><i>Example:</i> </p>
<pre>              while (cin.good())<br>              {  int x;<br>                 cin &gt;&gt; x;<br>                 if (cin.good()) a.push_back(x);<br>              }<br></pre>

<p><i>Pitfall: </i></p>
<pre>                 cin &gt;&gt; x; // &lt;--- may succeed and then encounter EOF<br>                 if (cin.good()) a.push_back(x);<br></pre>

<p>This code<i>may</i> miss the last element in the input file, if it is
directly followed by EOF. </p>

<p><i>Remedy:</i> Use <tt>fail()</tt>:</p>
<pre>              while (!cin.fail())<br>              {  int x;<br>                 cin &gt;&gt; x;<br>                 if (!cin.fail()) a.push_back(x);<br>              }<br></pre>

<p>The type conversion <tt>basic_ios ----&gt; void*</tt> is identical to
<tt>!fail(): </tt></p>
<pre>              while (cin)<br>              {  int x;<br>                 cin &gt;&gt; x;<br>                 if (cin) a.push_back(x);<br>              }<br></pre>

<p><i>Moral:</i> There are four stream test functions:<tt>good(), bad(),
eof()</tt>, and<tt>fail()</tt>. (Note that<tt>bad()</tt> does<i>not</i> mean
<tt>!good()</tt>.) Only one of them is useful: <tt>fail(). </tt> </p>
<hr>

<h3><b>Overloading pitfalls </b></h3>

<p><i>Example:</i> </p>
<pre>              class Complex<br>              {<br>              public:<br>                 Complex(double = 0, double = 0);<br>                 Complex operator+(Complex b) const;<br>                 Complex operator-(Complex b) const;<br>                 Complex operator*(Complex b) const;<br>                 Complex operator/(Complex b) const;<br>                 Complex operator^(Complex b) const;<br>                 // ...<br>              private:<br>                 double _re, _im;<br>              }; <br><br>              int main()<br>              {  Complex i(0, 1);<br>                 cout &lt;&lt; i^2 + 1; // i*i  is -1<br>                 return 0;<br>              }<br></pre>

<p>Why won't it print (0,0)? </p>

<p><i>Pitfall: </i> </p>

<p><tt></tt></p>
<pre>              cout &lt;&lt; i^2 + 1;<br></pre>

<p>Using the C/C++ operator precedence rules, we can add parentheses: </p>
<pre>              cout &lt;&lt; (i ^ (2 + 1));<br></pre>

<p>The <tt>^</tt> operator is weaker than <tt>+</tt> (but stronger than
<tt>&lt;&lt;</tt>). </p>

<p><i>Moral:</i> You cannot change the operator precedence when overloading
operators. Do not overload an operator if its precedence is not intuitive for
the problem domain. </p>

<p>The precedence of <tt>^</tt> is fine for XOR but not for raising to a power.
</p>
<hr>

<p><i>Example:</i> The stream classes support a type conversion <tt>basic_ios
----&gt; void* </tt> for testing if a stream is happy: </p>
<pre>              while (cin)<br>              {  int x;<br>                 cin &gt;&gt; x;<br>                 // ...<br>              }  <br></pre>

<p>Why convert to <tt>void*</tt>? A conversion to <tt>bool</tt> would seem to
make more sense. </p>
<pre>         <br>              template&lt;typename C, typename T = char_traits&lt;C&gt; &gt;     <br>              class basic_ios<br>              {  <br>              public:<br>                 // ...<br>                 operator bool() const<br>                 {  if (fail()) return false; <br>                    else return true;<br>                 }  <br>              private:<br>                 // ...<br>              }; <br><br></pre>

<p><i>Pitfall: </i> </p>
<pre>              while (cin)<br>              {  int x;<br>                 cin &lt;&lt; x;<br>                 // ...<br>              }  <br></pre>

<p>Note the typo--it should be <tt>cin &gt;&gt; x. </tt> </p>

<p>But<tt>cin &lt;&lt; x</tt> has an unintended meaning:<tt>cin.operator
bool()</tt>, converted to an <tt>int</tt> and shifted by <tt>x</tt> bits. </p>

<p><i>Moral:</i> Use conversion to<tt>void*</tt>, not conversion to
<tt>int</tt> or <tt>bool</tt>, to implement objects yielding truth values.
Unlike <tt>int</tt> or <tt>bool</tt>, <tt>void*</tt> have no legal operations
other than <tt>==</tt> comparison. </p>
<hr>

<p><i>Example:</i> An array class with a <tt>[]</tt> operator that <i>grows the
array on demand</i></p>
<pre>              class Array<br>              {  <br>              public:<br>                 Array();<br>                 ~Array();<br>                 Array(const Array&amp;);<br>                 Array&amp; operator=(const Array&amp;);<br>                 int&amp; operator[](int);<br>              private:<br>                 int _n; // current number of elements<br>                 int* _a; // points to heap array<br>              }; <br><br>              int&amp; Array::operator[](int i)<br>              {  if (i &gt; _n)<br>                 {  int* p = new int[i + 1];<br>                    for (int k = 0; k &lt; _n; k++) <br>                       p[k] = _a[k];<br>                    for (; k &lt; i; k++) p[k] = 0;<br>                    delete[] _a;<br>                    _a = p;<br>                    _n = i;<br>                 }<br>                 return _a[i];<br>              }  <br><br>              int main()<br>              {  Array a;<br>                 for (int s = 1; s &lt;= 100; s++)<br>                    a[s] = s * s;<br>                 return 0;<br>              }<br></pre>

<p><i>Pitfall: </i> </p>
<pre>              void swap(int&amp; x, int&amp; y)<br>              {  int temp = x;<br>                 x = y;<br>                 y = temp;<br>              }  <br><br>              int main()<br>              {  Array a;<br>                 a[3] = 9;<br>                 swap(a[3], a[4]);<br>                 return 0;<br>              }  <br></pre>

<p>The <tt>swap</tt> function gets references to <tt>a[3]</tt>, then to
<tt>a[4],</tt> but the second computation moves the array and invalidates the
first reference!<tt>a[4]</tt> is swapped with a wild reference. </p>

<p><i>Moral:</i> You cannot simultaneously relocate a memory block and export a
reference to it. </p>

<p>Either make <tt>[]</tt> not grow the array, or use a data structure in which
elements never move (i.e. a sequence of chunks, such as in
<tt>std::deque</tt>). </p>
<hr>

<h3><b>Exception pitfalls </b></h3>

<p><i>Example:</i> </p>
<pre>              void read_stuff(const char filename[])<br>              {  FILE* fp = fopen(filename, "r");<br>                 do_reading(fp);<br>                 fclose(fp);<br>              }  <br></pre>

<p>Why is that an "exception pitfall"? There aren't any exceptions in the code!
</p>

<p><i>Pitfall:</i> </p>
<pre>FILE* fp = fopen(filename, "r"); <br>do_reading(fp); <br>fclose(fp); // &lt;-- may never get here</pre>

<p>If <tt>do_reading</tt> throws an exception, or calls a function that throws
an exception, it never comes back!<tt>fp</tt> is never closed. </p>

<p><i>Moral:</i> Exception handling drastically alters control flow. You cannot
take it for granted that a function ever returns. </p>

<p><i>Remedy 1:</i> (popular but dumb) </p>
<pre>              void read_stuff(const char filename[])<br>              {  FILE* fp = fopen(filename, "r");<br>                 try <br>                    do_reading(fp);<br>                 catch(...)<br>                 {  fclose(fp);<br>                    throw;<br>                 }<br>                 fclose(fp);<br>              }  <br></pre>

<p><i>Remedy 2:</i> (smart) </p>
<pre>              void read_stuff(const char filename[])<br>              {  fstream fp(filename, ios_base::in);<br>                 do_reading(fp);<br>              }  <br></pre>

<p>Even if<tt>do_reading</tt> throws an exception, <tt>fp</tt> is closed by the
ifstream destructor. </p>

<p><i>Moral:</i> In code with exception handling (i.e. all C++ code starting in
1994), relinquish resources only in destructors! </p>
<hr>

<p><i>Example:</i> </p>
<pre>              double find_salary_increase(auto_ptr&lt;Employee&gt;);<br><br>              void do_stuff(const char name[])<br>              {  auto_ptr&lt;Employee&gt; pe = new Employee(name);<br>                    // can't use <br>                    // Employee* pe = new Employee(name)<br>                    // that's not not exception safe<br>                 double rate = find_salary_increase(pe);<br>                 pe-&gt;raise_salary(rate);<br>              }  <br></pre>

<p><i>Pitfall:</i> </p>
<pre>            find_salary_increase(<b>pe</b>);<br></pre>

<p>invokes the <i>copy constructor</i> of <tt>auto_ptr&lt;Employee&gt;</tt>
which <i>transfers ownership to the copy</i>. </p>

<p>Only one <tt>auto_ptr</tt> can own a heap object. The owning
<tt>auto_ptr</tt> calls the destructor when it goes out of scope. </p>

<p><i>Remedy:</i> Don't copy an <tt>auto_ptr</tt> into a function. </p>
<pre>              double find_salary_increase(Employee*);<br><br>              void do_stuff(const char name[])<br>              {  Employee* pe = new Employee(name);<br>                 auto_ptr&lt;Employee&gt; ape = pe; // use only for destruction<br>                 double rate = find_salary_increase(pe);<br>                 pe-&gt;raise_salary(rate);<br>              }  <br></pre>
<hr>

<h3><b>Container pitfalls </b></h3>

<p><i>Example:</i> A set of pointers</p>
<pre>              set&lt;Employee*&gt; staff;<br>              vector&lt;string&gt; names;<br>              for (int i = 0; i &lt; names.size(); i++)<br>                 staff.insert(new Employee(names[i]); <br></pre>

<p><i>Pitfall:</i> Ordered containers (<tt>set</tt>, <tt>map</tt>,
<tt>multiset</tt>, <tt>multimap</tt>) use <tt>&lt;</tt> for comparison. It is
assumed that <tt>&lt;</tt> is a <i>total ordering</i>. </p>

<p>[NOTE: Thanks to Liam Devine and Richard Smith for pointing out that this
out of date. Of course, the containers really use <code>less&lt;T&gt;</code>
for comparison, and The C++ standard Section 20.3.3.8 states: "For templates
<code>greater</code>, <code>less</code>, <code>greater_equal</code>, and
<code>less_equal</code>, the specializations for any pointer type yield a total
order, even if the built-in operators <code>&lt;</code>, <code>&gt;</code>,
<code>&lt;=</code>, <code>&gt;=</code> do not." When I wrote this, segmented
memory models were still widely used, and the C++ standard was still a work in
progress :-)]</p>

<p>In a set of pointers</p>
<pre>              set&lt;Employee*&gt; staff;<br></pre>

<p>the pointers are compared with <tt>&lt;</tt>.</p>

<p>Given two arbitrary <tt>Employee*</tt> pointers <tt>p</tt> and <tt>q</tt>,
is <tt>p &lt; q</tt> defined? <i>Only if they point to the same array</i>.</p>

<p>In a segmented memory model, only offsets are compared. Ex. <tt>p ==
0x740A0004</tt> and <tt>q == 0x7C1B0004</tt> compare identical.</p>

<p><i>Remedy:</i> (risky) Only write code for a flat memory space where pointer
word size == integer word size and comparison happens to be a total ordering.
</p>

<p><i>Remedy:</i> (tedious) Supply an ordering: </p>
<pre>              bool employee_ptr_less(const Employee* a, const Employee* b)<br>              {  return a-&gt;salary() &lt; b-&gt;salary();<br>              }<br><br>              set&lt;Employee*, bool (*)(const Employee*, const Employee*)&gt;<br>                 staff(employee_ptr_less);<br></pre>
<hr>

<p><i>Example:</i> Runaway iterators</p>
<pre>       list&lt;int&gt; a, b;<br>       // ...<br><br>       list&lt;int&gt;::iterator p<br>             = find(a.begin(), b.end(), 100); <br>       if (p != a.end()) b.push_back(*p);</pre>

<p><i>Pitfall:</i> </p>
<pre>             find(a.begin(), <b>b</b>.end(), 100); // oops, should have been a.end()<br></pre>

<p>To see why this code crashes dramatically, look at the implementation of
<tt>find</tt>:</p>
<pre>       template&lt;typename I, typename T&gt;<br>       I find(I from, I to, const T&amp; target)<br>       {  while (from != to &amp;&amp; *from != target)<br>             ++from;<br>                 return from; <br>       }<br></pre>

<p>When <tt>from</tt> reaches <tt>a.end()</tt>, <tt>*from</tt> and
<tt>++from</tt> are undefined.</p>

<p><i>Moral:</i> Iterators <i>don't know their state</i>. There is no reason
why a <tt>list&lt;int&gt;</tt> iterator couldn't know its state, but STL was
built with the objective to make iterators no more powerful than pointers into
a C array. Benefit: You can call the standard algorithms with C arrays:</p>
<pre>       int a[30];<br>       int* p = find(a, a + 30, 100);</pre>

<p>Drawback: Programming with iterators is <i>pointer-oriented,</i> not
object-oriented. </p>

<p><i>Remedy:</i> Use Safe STL (<a
href="http://www.horstmann.com/safestl.html">http://www.horstmann.com/safestl.html</a>)</p>
<hr>

<p><i>Example:</i> Muddled iterators</p>
<pre>       list&lt;int&gt; a;<br>       list&lt;int&gt; b;<br>       list&lt;int&gt;::iterator p = a.begin();<br><br>       a.insert(50);<br>       b.insert(100);<br><br>       b.erase(p);<br>       cout &lt;&lt; a.length() &lt;&lt; endl; // length is 1<br>       cout &lt;&lt; b.length() &lt;&lt; endl; // length is 0    <br></pre>

<p><i>Pitfall:</i> In</p>
<pre>       b.erase(p);<br></pre>

<p>the iterator <tt>p</tt> pointed inside <tt>a</tt>! The behavior is
undefined, but the standard STL implementation does the following: </p>
<ul>
  <li><tt>*p</tt> is erased from whatever list it happens to be in, just by
    following the forward and backwards link</li>
  <li>the length of <tt>b</tt> is decremented</li>
</ul>

<p><i>Moral:</i> Iterators <i>don't know their owner</i>.</p>

<p><i>Remedy:</i> Use Safe STL (<a
href="http://www.horstmann.com/safestl.html">http://www.horstmann.com/safestl.html</a>)</p>

<p></p>
</body>
</html>
